fix(falkordb): make the graph-DB push converge, batch its writes, add a repo-keyed delta (#3057) - #3069

Fixes #3057. Targets v8 directly — this is the upstream-mergeable version of the fix; see the note at the bottom.

The defect

push_to_falkordb is MERGE-only. Once graphify global add prunes a repo out of the global graph, a full re-push leaves every one of those nodes in the target permanently — graph.json and the database diverge and never converge back, no matter how many times you re-push. @Azeem1985 measured it: +1,250 nodes / +1,151 edges of surplus on a 75k-node global graph, and 25 of 25 sampled pruned ids still present after a full re-push.

While fixing it, a second problem surfaced that is not in the report. Measured here, 20 repos / 20k nodes / 20k edges:

OLD per-row full push    79.73s
NEW batched full push     2.90s   (27x)

The old path sent one query per node and one per edge, and matched edge endpoints with a label-free MATCH (a {id: $src}) that no index can serve — because nodes carried only their file-type label (:Python), there was no shared label to index. That is @kingjin94's #2258 diagnosis, and it is fixed here by giving every node :Entity alongside its file-type label and indexing n.id.

What changed

Batched writes on an indexed shared label. UNWIND in 1000-row batches, MERGE (n:Entity {id: row.id}) SET n:Python SET n += row, with CREATE INDEX FOR (n:Entity) ON (n.id). The file-type label is kept, not replaced, so existing queries keyed on it still work. A one-time backfill (MATCH (n) WHERE n.id IS NOT NULL AND NOT n:Entity SET n:Entity) adopts nodes written by an older graphify — without it, the MERGE switch would create a duplicate beside every existing node. There is a test asserting both labels survive.

--graph-name. The writer has always taken graph_name; the CLI never passed it, so every push through the CLI landed on the graphify key with no flag to send it elsewhere. You could not rehearse a push, run a staging graph beside production, or point two environments at one instance without dropping to Python. This is also what turned @Azeem1985's mistake into a 1.2M-node deletion.

--prune converges the target — nodes and edges. DETACH DELETE on a stale node takes its edges with it, but an edge dropped between two endpoints that both survive needs its own sweep; without it the surplus-edge half of the report lives on. Opt-in rather than default: --graph-name never existed, so some graphify keys genuinely hold several projects merged together, and a converging default would either silently delete the others (under the guard) or hard-refuse a routine re-push (over it). Neither is acceptable in a patch upgrade — @Azeem1985's framing, and it is better than my original reasoning.

An add-only push reports the drift it cannot fix. The core complaint in #3057 is that the divergence is silent, and that stays true on the default path if the only fix is a flag nobody passes. So a non-pruning push now counts the nodes it did not stamp — the same query the prune path deletes by, so there is one definition of drift rather than two that can disagree — and the CLI prints one line:

  note: target has 2 node(s) the source does not; --prune converges it.

Repo-keyed delta, via the new graphify global push (delta by default). This mirrors global_add's own contract instead of inventing one: the repo is already its unit of change — it prunes a repo whole, re-adds it whole, records a per-repo source_hash, and already returns skipped=True when that hash has not moved. The push now consumes the same signal.

Delta output is identical to a from-scratch load on node and edge counts.

Drift repair. Push state lives in the target as :GraphifyPushState nodes and is cross-checked against the target's own per-repo counts (one aggregate). A local ledger still reads clean after the database is wiped or a run half-lands, and the delta would skip forever — @Azeem1985's finding #2. There is a test that deletes part of a repo behind the push's back, leaves the hashes untouched, and asserts the repo is rebuilt.

Cross-repo edges survive a delta. global_add remaps external-library nodes onto whichever repo first contributed them, so a B -> A edge is owned by neither repo alone. Pruning A drops it, and re-adding only A's internal edges would erode cross-repo connectivity a little on every delta — silently. The delta buckets edges by either endpoint's repo. Pinned by a test.

Delete guards. Capped at 20% of the target unless --allow-shrink — the same "refuse to SILENTLY drop nodes" rule as the #479 build guard. Only net removal counts: a re-pushed repo is pruned and immediately re-added, so charging it would refuse any delta touching more than 20% of a small global graph; a repo that comes back smaller is charged the difference, which is what catches "the manifest says 2 nodes, the target holds 50,000". Deletes are paged with the LIMIT inside a WITH, because FalkorDB's LIMIT does not short-circuit an eager DELETE — ... DETACH DELETE n LIMIT $page deletes the whole label. That is @Azeem1985's finding #1, and the reason sits in a comment at the call site.

graphify global push. export --push resolves its source from a project's output directory, so the global graph — the one graph this entire contract is about — had no CLI push route at all.

Convergence, measured

OLD full re-push, after the source pruned   -> 20,240 nodes   converged=False
NEW prune re-push                           -> 20,000 nodes   converged=True

Tests

15 new integration tests against a live FalkorDB: both labels, add-only default, drift reporting, node convergence, edge-only convergence, prune idempotence, the guard refusing and not deleting, target isolation by --graph-name, delta skip / selective re-send / repo removal, cross-repo edge preservation, drift repair, and a foreign manifest being refused.

Verified they catch the bugs rather than just passing: all 15 fail on this commit's parent, all pass here (15 failed / 2 passed against the parent; the 2 are the pre-existing create/idempotence tests). Independently reproduced by @Azeem1985 on the rig that produced the original #3057 measurements. Full suite is +15 passing with no new failures (this environment has 557 pre-existing failures from missing tree-sitter grammars, identical before and after).

Deliberately not included

The Neo4j writer is untouched — byte-identical to the parent, verified by AST comparison. It has the same defects (per-row, unindexed, never deletes, no database parameter), but a fix cannot be verified without a Neo4j instance. The new flags are refused on export neo4j rather than silently ignored: accepting --prune there and doing nothing would report a converged push that converged nothing — the same class of lie the missing-label bug told. Follow-up when there is an instance to test against.

No CHANGELOG entry — this repo folds those at release (e.g. 1c6b3db), per @Azeem1985.

Streaming graph.json into the push instead of loading it through NetworkX is not in here. It is a real problem (@Azeem1985 reports a 1.87GB graph peaking ~5.3GB RSS and getting OOM-killed before a row went out), but it is a separate change to how export loads graphs and belongs in its own PR, the way it was originally offered.

Note on provenance

This work was first written against a FalkorDB-backend branch where graphify/store.py provides a GraphStore with a batched writer, and reviewed there by @Azeem1985 on the box that produced the original #3057 numbers (review). That version is not portable to v8, which has no store.py at all — every symbol it leaned on is absent, so a cherry-pick auto-merges four of five files and then ImportErrors on first use. This PR is the v8-native implementation of the same contract: the batched writer is hand-rolled in graphdb.py, and the delta buckets the in-memory graph by repo in two passes rather than querying the source. Same tests, same guards, same measured behaviour.

Relationship to #2312

#2312 proposes a Neo4j source-of-truth backend. This is not competing with it — it is the push-side piece that design needs regardless, and it is small enough to land independently.